-
Notifications
You must be signed in to change notification settings - Fork 1.4k
tools/docker: assorted updates for syz-env #5653
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
a-nogikh
commented
Jan 2, 2025
- Use sub-containers to parallelize/cache some build steps.
- Add a Spanner emulator binary -- it will be used in syz-cluster: initial code #5620
It allows for better caching/parallelization. Also, the resulting image is now ~100MB smaller.
This allows for better caching and parallelization.
DOCKER_BUILDKIT=1 ensures that the Docker builder parallelizes the build steps (whenever it's possible).
c26231e to
e7009b2
Compare
|
I'm hesitating to add spanner emulator as a dependency. I still believe mocking is a better way to go. |
|
Even if we generate mocks to test the higher level logic, we still want to test the code that actually interacts with the database (the |
|
You can specify the SQL command your code is expected to generate and send. Unit Test will check the command equality. The question then is "why is emulator better than the real system?". |
It also means that there's less trust to such tests unless we have proper full integration testing done at presubmit (and we don't). The reviewer, if they want to be sure, would have to actually deploy the PR on the server or recreate the DB in their Spanner instance and re-run the modified SQL queries. Otherwise, even though the code is "tested", in reality the code that passed all our checks can easily fail in production.
At the very least, it's faster (one command to run Go tests that takes less than a second vs minutes to push/rebuild/redeploy on Cloud and monitor the results) and more convenient during active development. K8S gives the flexibility to deploy in different environments, and so far I have really enjoyed having a local dev cluster on my workstation. I would also really like to decouple |
|
I am in favor of adding the emulator.
Mocks won't replace end-to-end testings with emulator b/c they will have smaller code coverage. Mocks are complementary to end-to-end tests, they are more unit-test style and allow to test implementation details. I see we can have either:
But we shouldn't have only mock based tests. If code does the right thing in the end, I don't care much what exact queries it issued, how many, and what results it received. But if it does not work in the end, but issued query sequence that matches some hard-coded golden sequence, that's not very useful. So I would go with merging the emulator. Later we may add mock-based tests (if we start missing bugs that can't be detected with end-to-end tests, and mocks show good additional-code-to-value ratio). |
|
@a-nogikh what is the proposed eventual testing strategy? It may be better to focus on this question. I'm using mocks for Spanner unit testing (in existing code and in the pending). You like to use emulator for similar unit testing. Mocks are faster, more flexible and will give you better coverage. You're able to simulate any external system failure. Emulator has limitations (one, two).
#4785 is a specific Datastore Emulator concern I have. |
|
These don't seem to be critical for our use cases.
That would work perfectly well for higher level tests that use
Mock tests would only check that the code behaves exactly as stated in tests, but that does not guarantee that the behavior is indeed correct.
Spanner emulator is a separate binary, so there should be no problems like that. |
tarasmadan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's try